Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughgRPC로 사용자 정보를 일괄 조회해 가입 폼 목록에 닉네임을 주입하도록 기능이 추가되었습니다. JoinInfo.of 시그니처가 닉네임을 받도록 변경되고, JoinMyInfo 레코드와 사용자 gRPC proto/클라이언트 설정이 추가되었습니다. (요약 50단어 이내) Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as JoinController
participant Service as ApplicationFormService
participant gRPC as UserQueryService (gRPC)
participant Result as FindJoinFormListResult
Client->>Controller: findGroupJoinList()
Controller->>Service: findJoinFormList()
Service->>Service: extract user IDs from joins
Service->>gRPC: GetUsers(GetUsersRequest{user_ids})
gRPC-->>Service: GetUsersResponse{users}
Service->>Service: build Map<userId, GetUserResponse>
Service->>Result: of(joinList, userMap)
Result-->>Service: FindJoinFormListResult
Service-->>Controller: Result
Controller-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/flipnote/group/application/port/in/result/FindJoinFormListResult.java`:
- Around line 17-18: The current code calls
userMap.get(join.getUserId()).getNickname() which throws NPE when the user is
missing; update the logic around where JoinInfo.of(join, user.getNickname()) is
returned to handle a null GetUserResponse from userMap (e.g., retrieve
GetUserResponse user = userMap.get(join.getUserId()); then if user == null
either supply a null-safe fallback nickname like "Unknown User"/empty string or
throw an explicit, descriptive exception), and pass that safe value into
JoinInfo.of; adjust any callers or tests if they expect an exception vs
fallback.
In
`@src/main/java/flipnote/group/application/service/ApplicationFormService.java`:
- Around line 107-112: The gRPC call in ApplicationFormService uses
userQueryServiceStub.getUsers with GetUsersRequest without any deadline, risking
an indefinitely blocking thread; change the call site to apply a deadline (e.g.,
call userQueryServiceStub.withDeadlineAfter(...) or use Context/Deadline
utilities before invoking getUsers) so the remote call times out, and wrap the
invocation in a try/catch that catches io.grpc.StatusRuntimeException and maps
it to a suitable domain exception (e.g., UserServiceUnavailableException or a
specific ApplicationFormServiceException) to preserve error semantics; ensure
you reference the existing GetUsersRequest, userQueryServiceStub, and getUsers
symbols when making the change.
In `@src/main/proto/user.proto`:
- Around line 18-23: The GetUserResponse message currently exposes unnecessary
PII (email); update the proto to tighten the response for nickname mapping by
removing the email and profile_image_url fields and keeping only id and nickname
in GetUserResponse, or alternatively create a new lightweight RPC/message (e.g.,
GetUserNicknameResponse) that contains just int64 id and string nickname for the
mapping use-case; update any references to GetUserResponse in server handlers
and clients to use the new/trimmed message to avoid leaking email across
services.
- Line 7: The proto package declaration "package user_query;" in user.proto
doesn't match its repository location and triggers PACKAGE_DIRECTORY_MATCH
linting; fix by either moving the user.proto file into a directory that
corresponds to the package namespace (so its filesystem path matches package
user_query) or by changing the package declaration in user.proto from
"user_query" to the correct package name that matches the file's current
directory; ensure the package token exactly matches the target directory name
and re-run the proto linter.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/flipnote/group/adapter/in/web/JoinController.javasrc/main/java/flipnote/group/api/dto/response/FindMyJoinListResponseDto.javasrc/main/java/flipnote/group/application/port/in/result/FindJoinFormListResult.javasrc/main/java/flipnote/group/application/port/in/result/FindMyJoinListResult.javasrc/main/java/flipnote/group/application/service/ApplicationFormService.javasrc/main/java/flipnote/group/domain/model/join/JoinInfo.javasrc/main/java/flipnote/group/domain/model/join/JoinMyInfo.javasrc/main/java/flipnote/group/global/config/GrpcClientConfig.javasrc/main/proto/user.protosrc/main/resources/application.yml
💤 Files with no reviewable changes (1)
- src/main/java/flipnote/group/adapter/in/web/JoinController.java
src/main/java/flipnote/group/application/port/in/result/FindJoinFormListResult.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/service/ApplicationFormService.java
Show resolved
Hide resolved
| option java_outer_classname = "UserQueryProto"; | ||
| option java_multiple_files = true; | ||
|
|
||
| package user_query; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if proto directory exists and list files
echo "=== Proto files structure ==="
if [ -d "src/main/proto" ]; then
find src/main/proto -type f -name "*.proto" | head -20
else
echo "Directory src/main/proto not found"
fi
echo ""
echo "=== Running package-directory mismatch check ==="
python3 <<'PY'
from pathlib import Path
root = Path("src/main/proto")
if not root.exists():
print(f"Root path {root} does not exist")
else:
violations = []
for p in sorted(root.rglob("*.proto")):
pkg = None
try:
content = p.read_text(encoding="utf-8")
for line in content.splitlines():
s = line.strip()
if s.startswith("package ") and s.endswith(";"):
pkg = s[len("package "):-1].strip()
break
except Exception as e:
print(f"Error reading {p}: {e}")
continue
if not pkg:
print(f"No package found in {p}")
continue
expected_dir = root / pkg.replace(".", "/")
if p.parent != expected_dir:
violations.append({
'file': str(p),
'package': pkg,
'expected_dir': str(expected_dir),
'actual_dir': str(p.parent)
})
if violations:
print(f"Found {len(violations)} PACKAGE_DIRECTORY_MATCH violation(s):")
for v in violations:
print(f" - File: {v['file']}")
print(f" Package: {v['package']}")
print(f" Expected dir: {v['expected_dir']}")
print(f" Actual dir: {v['actual_dir']}")
else:
print("No PACKAGE_DIRECTORY_MATCH violations found")
PYRepository: FlipNoteTeam/FlipNote-Group
Length of output: 675
🏁 Script executed:
cat src/main/proto/user.protoRepository: FlipNoteTeam/FlipNote-Group
Length of output: 682
package 선언과 파일 경로가 맞지 않아 proto lint가 실패합니다.
package user_query;면 Buf 규칙상 파일은 src/main/proto/user_query/ 디렉토리 하위에 있어야 합니다. 현재 위치(src/main/proto/user.proto)는 PACKAGE_DIRECTORY_MATCH 오류를 유발합니다. 파일을 src/main/proto/user_query/user.proto로 이동하거나 패키지 선언을 수정하세요.
🧰 Tools
🪛 Buf (1.65.0)
[error] 7-7: Files with package "user_query" must be within a directory "user_query" relative to root but were in directory "src/main/proto".
(PACKAGE_DIRECTORY_MATCH)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/proto/user.proto` at line 7, The proto package declaration "package
user_query;" in user.proto doesn't match its repository location and triggers
PACKAGE_DIRECTORY_MATCH linting; fix by either moving the user.proto file into a
directory that corresponds to the package namespace (so its filesystem path
matches package user_query) or by changing the package declaration in user.proto
from "user_query" to the correct package name that matches the file's current
directory; ensure the package token exactly matches the target directory name
and re-run the proto linter.
Comment on lines
+18
to
+23
| message GetUserResponse { | ||
| int64 id = 1; | ||
| string email = 2; | ||
| string nickname = 3; | ||
| string profile_image_url = 4; | ||
| } |
There was a problem hiding this comment.
현재 사용 목적 대비 응답 스키마가 과도하게 넓습니다.
닉네임 매핑 용도인데 email까지 함께 전달되어 서비스 간 PII 노출면이 커집니다. id, nickname 중심의 축소 응답(또는 별도 RPC)으로 분리하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/proto/user.proto` around lines 18 - 23, The GetUserResponse message
currently exposes unnecessary PII (email); update the proto to tighten the
response for nickname mapping by removing the email and profile_image_url fields
and keeping only id and nickname in GetUserResponse, or alternatively create a
new lightweight RPC/message (e.g., GetUserNicknameResponse) that contains just
int64 id and string nickname for the mapping use-case; update any references to
GetUserResponse in server handlers and clients to use the new/trimmed message to
avoid leaking email across services.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements